Keep codex account file reads off the menu-build path#1401
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-run |
|
🦞👀 Command router queued. I will update this comment with the next step. |
populateMenu -> codexAccountMenuDisplay loaded the codex account reconciliation snapshot synchronously whenever the 2s freshness cache had lapsed, paying auth.json reads, JWT parsing, and SHA256 fingerprinting on the main thread inside menu open and tracking. Menu display now tolerates a stale cached snapshot and revalidates the cache off the menu-build path; account changes land on the next rebuild.
|
Local validation on Result:
The new menu-display snapshot coverage passed, including Scope note: I did not run a packaged app UI sample here; this is focused local regression validation for the menu-account snapshot path. |
|
Checked the current merge conflict against The conflict is limited to the So the rebase looks low-risk: keep this PR’s changelog item under |
steipete
force-pushed
the
perf/menu-account-snapshot-stale
branch
from
53cd615 to
f1c1b6c
Compare
|
Codex review: needs real behavior proof before merge. Reviewed June 10, 2026, 11:13 PM ET / 03:13 UTC. Summary Reproducibility: yes. from current-main source and the reported field samples: once the short reconciliation cache expires, Codex menu construction can synchronously enter account discovery, file reading, JWT parsing, and fingerprinting. A live reproduction was not performed during this read-only review. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep the guarded asynchronous projection design and verify a packaged build with redacted sampling that menu opening and provider switching avoid reconciliation while account add, removal, and active-source changes appear on the next safe menu rebuild. Do we have a high-confidence way to reproduce the issue? Yes from current-main source and the reported field samples: once the short reconciliation cache expires, Codex menu construction can synchronously enter account discovery, file reading, JWT parsing, and fingerprinting. A live reproduction was not performed during this read-only review. Is this the best way to solve the issue? Yes. Separating a side-effect-free menu projection from the authoritative reconciliation snapshot directly removes blocking work from the menu path while preserving the existing account model and non-menu reconciliation behavior. AGENTS.md: found and applied where relevant. Codex review notes: reasoning high; reviewed against 02c94032c79a. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Runtime proof — packaged app at current head Per the review ask: redacted Setup
Method
Result: zero reconciler frames in every capture Every capture demonstrably contains live menu tracking (so the zeros are not from sampling a closed menu): Representative main-thread stack from The switching capture ( Account data still renders correctly in the menu (accounts list, cost rows, switcher all populated), so the cached-projection path is serving real data while revalidation stays off the menu path. |
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Landed in The final rewrite keeps Codex account reconciliation and menu identity reads off menu tracking, adds source/generation guards around background revalidation, and covers override-cache and managed-account invalidation regressions. Proof: 55 focused tests passed, |
3 participants
Summary
Keep Codex account discovery,
auth.jsonreads, JWT parsing, and auth fingerprint hashing off the menu-build and menu-tracking paths.Problem
Sampling the provider switch path showed synchronous account reconciliation below:
The same work was also reachable through the menu identity signature. Once the short-lived reconciliation cache expired, opening or rebuilding the menu could perform file and crypto work on the main actor.
Change
UsageStore.accountInfo.Validation
swift test --filter 'CodexAccountMenuDisplaySnapshotTests|CodexAccountReconciliationTests|StatusMenuCodexSwitcherTests'(55 tests, 3 suites, pass)make check(SwiftFormat clean; SwiftLint 0 violations)git diff --checkRuntime Evidence
The original field samples captured repeated
DefaultCodexAccountReconciler.loadSnapshotandloadLiveSystemAccountframes belowpopulateMenu. The rewritten path has no reconciliation call from menu projection reads or menu identity calculation; tests use a blocking loader probe to verify menu open remains non-blocking and the loader executes off the main thread.Part of #1387.